-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: add configurable timeout for LM Studio provider #6523
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add lmStudioTimeoutSeconds setting to provider schema (30-3600 seconds) - Implement AbortController with configurable timeout in LM Studio handler - Default timeout set to 600 seconds (10 minutes) for local model scenarios - Add comprehensive timeout tests including AbortError handling - Provide user-friendly timeout error messages with troubleshooting tips Fixes #6521
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code is like debugging in production - technically possible but morally questionable. I've reviewed the changes and left some suggestions inline to improve the implementation.
| let timeoutId: NodeJS.Timeout | undefined | ||
|
|
||
| // Get timeout from settings or use default (10 minutes) | ||
| const timeoutSeconds = this.options.lmStudioTimeoutSeconds ?? LMSTUDIO_DEFAULT_TIMEOUT_SECONDS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intentional that LM Studio has a configurable timeout while Bedrock has a hardcoded 10-minute timeout? This inconsistency might confuse users who work with both providers. Consider either making Bedrock's timeout configurable too, or documenting why they differ.
| const timeoutMs = timeoutSeconds * 1000 | ||
|
|
||
| try { | ||
| timeoutId = setTimeout(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The timeout setup logic is duplicated between createMessage and completePrompt. Could we extract this into a helper method like createAbortControllerWithTimeout(timeoutSeconds: number)?
| timeoutId = setTimeout(() => { | |
| private createAbortControllerWithTimeout(timeoutSeconds: number): { controller: AbortController; timeoutId: NodeJS.Timeout } { | |
| const controller = new AbortController() | |
| const timeoutId = setTimeout(() => { | |
| controller.abort() | |
| }, timeoutSeconds * 1000) | |
| return { controller, timeoutId } | |
| } |
| // Check if this is an abort error (timeout) | ||
| if (error instanceof Error && error.name === "AbortError") { | ||
| throw new Error( | ||
| `LM Studio request timed out after ${timeoutSeconds} seconds. This can happen with large models that need more processing time. Try increasing the timeout in LM Studio settings or use a smaller model.`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When logging timeout errors, could we include the actual timeout value used? This would help with debugging:
| `LM Studio request timed out after ${timeoutSeconds} seconds. This can happen with large models that need more processing time. Try increasing the timeout in LM Studio settings or use a smaller model.`, | |
| throw new Error( | |
| `LM Studio request timed out after ${timeoutSeconds} seconds (configured: ${this.options.lmStudioTimeoutSeconds || 'default'}). This can happen with large models that need more processing time. Try increasing the timeout in LM Studio settings or use a smaller model.`, | |
| ) |
| lmStudioBaseUrl: z.string().optional(), | ||
| lmStudioDraftModelId: z.string().optional(), | ||
| lmStudioSpeculativeDecodingEnabled: z.boolean().optional(), | ||
| lmStudioTimeoutSeconds: z.number().min(30).max(3600).optional(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is 30 seconds a reasonable minimum for very large models? Some models might need more time just to start processing. Consider if 60 seconds would be a safer minimum to prevent user frustration.
|
This should be a general setting for all providers, closing for now |
Fixes #6521
Summary
This PR adds configurable timeout support for the LM Studio provider to address timeout issues when using large local models that require more processing time, especially when split between GPU and CPU.
Changes
Core Implementation
lmStudioTimeoutSecondssetting (30-3600 seconds range)AbortControllerwith configurable timeout for both streaming and non-streaming requestsTesting
Technical Details
The implementation follows the same pattern used in the Bedrock provider:
AbortControllerto manage request timeoutsUser Impact
Users experiencing timeout issues with LM Studio (especially with large models like Deepseek R1 14b/32b, Qwen2.5 Coder Instruct 32b) can now:
Testing
All tests pass, including new timeout-specific test cases.
Important
Adds configurable timeout for LM Studio provider with
AbortController, defaulting to 600 seconds, and enhances error handling and testing.lmStudioTimeoutSecondstolmStudioSchemainprovider-settings.ts(30-3600 seconds range).AbortControllerinLmStudioHandlerinlm-studio.tsfor request timeouts.lmstudio.spec.tsfor default and custom timeout behavior.AbortErrorhandling andAbortSignalusage in requests.This description was created by
for a694dc5. You can customize this summary. It will automatically update as commits are pushed.